Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make stdio configurable #102

Merged
merged 4 commits into from
Mar 14, 2020
Merged

make stdio configurable #102

merged 4 commits into from
Mar 14, 2020

Conversation

cjihrig
Copy link
Collaborator

@cjihrig cjihrig commented Mar 13, 2020

This PR adds options to uvwasi_init() to allow setting the stdio file descriptors. This will allow embedders like Node.js to run multiple WASI applications in the same process without sharing stdio.

Unfortunately, Windows doesn't seem to like the use of the names stdin, stdout, and stderr as struct members, which is why I went with in, out, and err.

Simplify the signature of uvwasi_fd_table_init() to make it
easier to configure stdio.
This commit refactors the code that configures the stdio
descriptors. Instead of setting them up in a loop, use a
function. This will make it simpler to add the stdio as
configurable options.
This commit requires the stdio file descriptors to be explicitly
defined. This allows embedders to run multiple WASI applications
in the same process (assuming they also monkey patch the
uvwasi_proc_exit() functionality to not force the process to
exit).
Windows doesn't seem to like the use of stdin, stdout, or stderr
as struct members.
@cjihrig
Copy link
Collaborator Author

cjihrig commented Mar 14, 2020

Tested these changes in the Node test suite (on macOS) and everything seems fine. Thanks for the review Anna.

@cjihrig cjihrig merged commit 28f2e31 into master Mar 14, 2020
@cjihrig cjihrig deleted the stdio branch March 14, 2020 18:50
This was referenced Mar 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants